-
Notifications
You must be signed in to change notification settings - Fork 0
feat: conditional authentication via discovery service #427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add getDiscovery() method to ObjectStackAdapter - Create useDiscovery hook for React components - Update AuthProvider to support enabled prop for bypassing auth - Create ConditionalAuthWrapper component to check server auth status - Update console App to use conditional auth wrapper Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
- Add test for AuthProvider with enabled=false (guest mode) - Add test for AuthProvider with enabled=true (normal auth) - Add test for guest session token creation - Fix TypeScript error in AuthProvider (expiresAt Date type) Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
- Add documentation for discovery property access in ObjectStackAdapter - Import and use DiscoveryInfo type in ConditionalAuthWrapper for type safety - Add comment explaining empty baseUrl in ConditionalAuthWrapper Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import { describe, it, expect, beforeEach } from 'vitest'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 17 hours ago
In general, unused imports should be removed to keep the code clear and avoid confusion. Here, the beforeEach symbol is imported but not used anywhere in the test file, so the safest and cleanest fix is to remove beforeEach from the import list.
Concretely, in packages/auth/src/__tests__/AuthProvider.disabled.test.tsx, update the import from vitest on line 9 to only import describe, it, and expect. No other changes are necessary: no additional methods, definitions, or imports are required, and the existing tests will continue to run as before.
-
Copy modified line R9
| @@ -6,7 +6,7 @@ | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import { describe, it, expect, beforeEach } from 'vitest'; | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { render, screen, waitFor } from '@testing-library/react'; | ||
| import { AuthProvider } from '../AuthProvider'; | ||
| import { useAuth } from '../useAuth'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds conditional authentication support by reading server discovery info to decide whether the Console should run with full auth or auto-guest mode when services.auth.enabled === false.
Changes:
- Adds
getDiscovery()to the ObjectStack data adapter and auseDiscovery()hook to expose discovery data +isAuthEnabled. - Extends
AuthProviderwithenabled={false}to bootstrap a guest session. - Updates Console to use a new
ConditionalAuthWrapperand documents the behavior inCONDITIONAL_AUTH.md.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/hooks/useDiscovery.ts | New hook + DiscoveryInfo type for reading discovery via SchemaRendererContext dataSource |
| packages/react/src/hooks/index.ts | Re-exports useDiscovery from the hooks barrel |
| packages/data-objectstack/src/index.ts | Adds ObjectStackAdapter.getDiscovery() (reads cached discovery from client) |
| packages/auth/src/AuthProvider.tsx | Adds enabled prop and guest-mode session bootstrapping |
| packages/auth/src/tests/AuthProvider.disabled.test.tsx | Adds tests for enabled={false} guest-mode behavior |
| apps/console/src/components/ConditionalAuthWrapper.tsx | New wrapper that fetches discovery and selects enabled/disabled AuthProvider |
| apps/console/src/App.tsx | Switches Console root auth wrapper from AuthProvider to ConditionalAuthWrapper |
| CONDITIONAL_AUTH.md | New documentation describing discovery structure, guest mode, and usage |
| import { ObjectStackAdapter } from './dataSource'; | ||
| import { AuthProvider } from '@object-ui/auth'; | ||
| import { LoadingScreen } from './components/LoadingScreen'; |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The relative imports are incorrect for a file located in src/components: ./dataSource and ./components/LoadingScreen do not exist from this directory and will break compilation. Import the adapter from ../dataSource and LoadingScreen from ./LoadingScreen (or the correct local path).
| import { ObjectStackAdapter } from './dataSource'; | |
| import { AuthProvider } from '@object-ui/auth'; | |
| import { LoadingScreen } from './components/LoadingScreen'; | |
| import { ObjectStackAdapter } from '../dataSource'; | |
| import { AuthProvider } from '@object-ui/auth'; | |
| import { LoadingScreen } from './LoadingScreen'; |
| // If auth is disabled, automatically set as authenticated with a guest user | ||
| const isAuthenticated = enabled | ||
| ? user !== null && session !== null | ||
| : true; |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When enabled === false, isAuthenticated is forced to true even before the guest user/session state is set. This can cause a transient render where AuthGuard treats the user as authenticated while user is still null (e.g., role checks are skipped because they are gated on user). Consider initializing guest user/session synchronously (state initializers) and deriving isAuthenticated from user && session, or keep isLoading true until the guest state is fully set.
| * | ||
| * Note: This accesses an internal property of the ObjectStackClient. | ||
| * The discovery data is populated during client.connect() and cached. | ||
| * | ||
| * @returns Promise resolving to discovery data, or null if not connected | ||
| */ | ||
| async getDiscovery(): Promise<unknown | null> { | ||
| try { | ||
| // Ensure we're connected first | ||
| await this.connect(); | ||
|
|
||
| // Access discovery data from the client | ||
| // The ObjectStackClient caches discovery during connect() | ||
| // This is an internal property, but documented for this use case | ||
| // @ts-expect-error - Accessing internal discovery property | ||
| return this.client.discovery || null; | ||
| } catch { |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getDiscovery() relies on an internal ObjectStackClient property (client.discovery) behind a @ts-expect-error and silently returns null on any failure. This is brittle against upstream client changes and makes debugging discovery failures hard. Prefer exposing a public discovery getter in @objectstack/client, or store the discovery result inside the adapter during connect() and return that; if errors are intentionally swallowed, consider at least logging (in dev) or returning a typed error alongside the result.
| * | |
| * Note: This accesses an internal property of the ObjectStackClient. | |
| * The discovery data is populated during client.connect() and cached. | |
| * | |
| * @returns Promise resolving to discovery data, or null if not connected | |
| */ | |
| async getDiscovery(): Promise<unknown | null> { | |
| try { | |
| // Ensure we're connected first | |
| await this.connect(); | |
| // Access discovery data from the client | |
| // The ObjectStackClient caches discovery during connect() | |
| // This is an internal property, but documented for this use case | |
| // @ts-expect-error - Accessing internal discovery property | |
| return this.client.discovery || null; | |
| } catch { | |
| * | |
| * Note: This reads discovery data cached on the underlying ObjectStackClient | |
| * during client.connect(). If discovery cannot be obtained, this method | |
| * returns null. In development builds, failures are logged to the console | |
| * to aid debugging. | |
| * | |
| * @returns Promise resolving to discovery data, or null if not connected | |
| */ | |
| async getDiscovery(): Promise<unknown | null> { | |
| try { | |
| // Ensure we're connected first | |
| await this.connect(); | |
| // Access discovery data from the client. | |
| // The ObjectStackClient caches discovery during connect(). | |
| const discovery = (this.client as { discovery?: unknown }).discovery; | |
| return discovery ?? null; | |
| } catch (error) { | |
| if (process.env.NODE_ENV === 'development') { | |
| // eslint-disable-next-line no-console | |
| console.warn( | |
| '[ObjectUI][ObjectStackDataSource] Failed to get discovery information from ObjectStack client:', | |
| error, | |
| ); | |
| } |
| [key: string]: any; | ||
| }; | ||
|
|
||
| /** API capabilities */ | ||
| capabilities?: string[]; | ||
|
|
||
| /** Additional discovery metadata */ | ||
| [key: string]: any; |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DiscoveryInfo uses any for the services index signature and top-level index signature ([key: string]: any). This loses type safety and is inconsistent with other repo types (e.g., AuthUser uses [key: string]: unknown). Prefer unknown/Record<string, unknown> (and narrow where needed) to keep consumers type-safe.
| [key: string]: any; | |
| }; | |
| /** API capabilities */ | |
| capabilities?: string[]; | |
| /** Additional discovery metadata */ | |
| [key: string]: any; | |
| [key: string]: unknown; | |
| }; | |
| /** API capabilities */ | |
| capabilities?: string[]; | |
| /** Additional discovery metadata */ | |
| [key: string]: unknown; |
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import { describe, it, expect, beforeEach } from 'vitest'; |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import: beforeEach is imported from vitest but never used in this test file. Please remove it to keep the test code clean and avoid unused-import lint/typecheck failures if enabled.
| import { describe, it, expect, beforeEach } from 'vitest'; | |
| import { describe, it, expect } from 'vitest'; |
Enables automatic detection of auth service availability via discovery endpoint. When
auth.enabled === false, console bypasses login and operates in guest mode—suitable for dev/demo environments without authentication infrastructure.Changes
Data Layer
ObjectStackAdapter.getDiscovery()exposes server discovery data (capabilities, service status)React Layer
useDiscovery()hook provides{ discovery, isAuthEnabled, isLoading, error }AuthProvideracceptsenabled={false}to auto-authenticate as guest userConsole App
ConditionalAuthWrapperfetches discovery on mount, wraps with appropriate auth provider{ id: 'guest', name: 'Guest User', token: 'guest-token' }Example
Discovery response structure:
{ "services": { "auth": { "enabled": false, // triggers guest mode "status": "unavailable" } } }Security
Defaults to auth enabled on discovery failure. Guest mode requires explicit
auth.enabled = falsefrom server.Documentation
See
CONDITIONAL_AUTH.mdfor API reference and usage patterns.Original prompt
Start implementation
The user has attached the following file paths as relevant context:
- .github/copilot-instructions.md
[Chronological Review: The conversation began with a user request regarding the behavior of the authentication interface in the @objectstack/client when the discovery service indicates that authentication is disabled. The user provided a JSON snippet showing the service status. The conversation has focused on whether to load the login authentication interface in environments where authentication is not enabled.][Intent Mapping: The user explicitly requested guidance on whether to load the login authentication interface when the discovery response indicates that "auth.enabled" is false. The context provided suggests a consideration for development or demonstration environments.]
[Technical Inventory: The conversation involves the @objectstack/client, specifically focusing on the authentication service and its configuration. The user provided a JSON structure detailing the service's status, including fields like "enabled," "status," and "message."]
[Code Archaeology: The user referenced a JSON snippet that includes the "services" object with an "auth" property. This snippet is crucial for understanding the current state of the authentication service.]
[Progress Assessment: The conversation is currently in the inquiry phase, with the user seeking clarification on a specific implementation decision regarding the authentication interface based on the service status.]
[Context Validation: The user’s request is clear, and the context is well-defined, focusing on the implications of the authentication service's status for user interface behavior.]
[Recent Commands Analysis: The last agent command was the user request regarding the authentication service behavior. The tool results are not applicable here as the conversation has not involved tool executions but rather a direct inquiry.]
1. Conversation Overview: - Primary Objectives: The user asked, "如果 discovery 返回 auth.enabled === false 表示服务端没有认证服务,可能是开发或演示环境,此时是否建议不加载登录认证界面?" (If discovery returns auth.enabled === false, indicating that the server does not have an authentication service, possibly in a development or demonstration environment, should the login authentication interface not be loaded?) - Session Context: The conversation has focused on the implications of the authentication service's status for the user interface, particularly in development or demonstration scenarios. - User Intent Evolution: The user is seeking a decision-making guideline based on the service's authentication status.- Technical Foundation:
- @objectstack/client: This is the client library being discussed, which interacts with the authentication service.
- JSON Structure: The user provided a JSON snippet detailing the authentication service's status.
- Codebase Status:
- JSON Snippet:
- Purpose: To convey the current state of the authentication service.
- Current State: The snippet indicates that the authentication service is unavailable and suggests installing an auth plugin to enable it.
- Key Code Segments: The relevant fields include "enabled," "status," and "message."
- Dependencies: This snippet is crucial for determining the behavior of the user interface based on the authentication service's availability.
- Problem Resolution:
- Issues Encountered: The user is uncertain about the appropriate action to take when the authentication service is disabled.
- Solutions Implemented: No solutions have been implemented yet; the user is seeking advice.
- Debugging Context: There are no ongoing troubleshooting efforts as the conversation is focused on decision-making.
- Lessons Learned: The conversation highlights the need for clear guidelines on UI behavior based on service status.
- Progress Tracking:
- Completed Tasks: No tasks have been completed yet; the conversation is still in the inquiry phase.
- Partially Complete Work: The user is actively seeking clarification on the authentication interface behavior.
- Validated Outcomes: No features or code have been confirmed working through testing yet.
- Active Work State:
- Current Focus: The user is focused on understanding whether to load the login authentication interface based on the authentication service's status.
- Recent Context: The last few exchanges have revolved around the implications of the authentication service being disabled.
- Working Code: The JSON snippet provided by the user is the primary focus of the discussion.
- Immediate Context: The user is addressing the specific question of UI behavior in relation to the authentication service status.
- Recent Operations:
- Last Agent Commands: The user requested guidance on the authentication interface behavior based on the service status.
- Tool Results Summary: No tool results were generated as the conversation has not involved tool executions.
- Pre-Summary State: The agent was actively dis...
Created from VS Code.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.